Skip to content

Conversation

TrigamDev
Copy link
Contributor

@TrigamDev TrigamDev commented Apr 25, 2025

Summary

Text fields will now replace all* URLs in their text with clickable links. Text fields have also been changed to support links and be able to open them when clicked.

Tasks Completed

  • Platforms Tested:
    • Windows x86
    • Windows ARM
    • macOS x86
    • macOS ARM
    • Linux x86
    • Linux ARM
  • Tested For:
    • Basic functionality
    • PyInstaller executable

Closes #506

@CyanVoxel CyanVoxel added Type: Enhancement New feature or request Type: QoL A quality of life (QoL) enhancement or suggestion Type: UI/UX User interface and/or user experience Status: Review Needed A review of this is needed labels Apr 25, 2025
@CyanVoxel CyanVoxel moved this to 👀 In review in TagStudio Development May 4, 2025
@CyanVoxel CyanVoxel added this to the Alpha v9.5.3 milestone May 4, 2025
@python357-1
Copy link
Collaborator

Confirmed working on Linux x86 and macOs ARM. Not sure if there is a way to do this but maybe you could make it so that I don't have to type "http(s)://" in front of my URLs? ex: "tagstud.io" isn't clickable, but "https://tagstud.io" is.
Screenshot 2025-06-03 at 4 34 49 PM

@TrigamDev
Copy link
Contributor Author

Unfortunately, that's not exactly the easiest to do without creating a LOT of false positives. Requiring the protocol seems to be common for most apps/websites, so that's what I went with.

@Computerdores
Copy link
Collaborator

Unfortunately, that's not exactly the easiest to do without creating a LOT of false positives. Requiring the protocol seems to be common for most apps/websites, so that's what I went with.

one approach might be looking at the tld of the candidate domain, because which ones are valid is quite limited (there is a wikipedia page that lists all of them)

@TrigamDev
Copy link
Contributor Author

TrigamDev commented Jun 3, 2025

Hm, that's true, though there's still the possibility of it running into false positives if the user happens to include a period anywhere in a text field, which isn't ideal

@Computerdores
Copy link
Collaborator

definitely, but this kind of automatic detection in text always does.

For that reason I personally favor manually marking things as links with markdown (having markdown WYSIWYG for notes is something that has been discussed before as well, not sure if there is commitment on that yet though), however I am not sure what would be best for the average joe (I write most documents in LaTeX or Markdown so I am not exactly representative of the average person in that regard)

@python357-1
Copy link
Collaborator

I guess I didn't realize this also does it in text fields but I was talking about the URL field specifically. Maybe we could have full protocol checking in regular text fields and no checking in URL fields?

@TrigamDev
Copy link
Contributor Author

After looking into it a bit, it seems like there's over 1k TLDs that would have to be checked for, which.. isn't very feasible to check for

@TrigamDev
Copy link
Contributor Author

definitely, but this kind of automatic detection in text always does.

For that reason I personally favor manually marking things as links with markdown (having markdown WYSIWYG for notes is something that has been discussed before as well, not sure if there is commitment on that yet though), however I am not sure what would be best for the average joe (I write most documents in LaTeX or Markdown so I am not exactly representative of the average person in that regard)

This could be a good solution, though it would have to be explained to the user somehow, as some people probably aren't very familiar with markdown

@TrigamDev
Copy link
Contributor Author

definitely, but this kind of automatic detection in text always does.
For that reason I personally favor manually marking things as links with markdown (having markdown WYSIWYG for notes is something that has been discussed before as well, not sure if there is commitment on that yet though), however I am not sure what would be best for the average joe (I write most documents in LaTeX or Markdown so I am not exactly representative of the average person in that regard)

This could be a good solution, though it would have to be explained to the user somehow, as some people probably aren't very familiar with markdown

Hell, if going that route, there's the possibility of just going ahead and adding markdown rendering support to text fields. Though, that's a whole other can of worms.

@TrigamDev
Copy link
Contributor Author

At least for now though, I think the current implementation is fine enough for the time being, unless any problems are found

@python357-1
Copy link
Collaborator

I don't think we have to go that far, just a regex (like this, for example: /^https?:\/\/(?:www\.)?[-a-zA-Z0-9@:%._\+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b(?:[-a-zA-Z0-9()@:%_\+.~#?&\/=]*)$/) for text fields and any text in a URL field be clickable as a link.

@TrigamDev
Copy link
Contributor Author

I don't think we have to go that far, just a regex (like this, for example: /^https?:\/\/(?:www\.)?[-a-zA-Z0-9@:%._\+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b(?:[-a-zA-Z0-9()@:%_\+.~#?&\/=]*)$/) for text fields and any text in a URL field be clickable as a link.

After checking it really quickly, it doesn't match the URL you're trying to match
Screenshot 2025-06-03 182916

@python357-1
Copy link
Collaborator

Yeah, I think going without protocol in text fields would add too many false positives, like you said. Putting the full protocol in text fields and not having to do it in URL fields feels like a good compromise

@TrigamDev
Copy link
Contributor Author

In that case, I think a specific URL widget would have to be added, as currently the URL-related fields just use the text box widget

@python357-1
Copy link
Collaborator

In that case I'm just going to say it's good as is once the ruff errors are fixed.

@TrigamDev
Copy link
Contributor Author

Thing is, the Ruff errors aren't related to my commit, they're from the source. Fixing them would mean editing random source code that's entirely unrelated to the feature itself, and should be left to a PR focused on fixing them.

@TrigamDev
Copy link
Contributor Author

Ok nvm, apparently I misremembered

@CyanVoxel CyanVoxel removed the Status: Review Needed A review of this is needed label Jun 4, 2025

# Regex from https://stackoverflow.com/a/6041965
def linkify(text: str):
url_pattern = r"(http|ftp|https):\/\/([\w_-]+(?:(?:\.[\w_-]+)+))([\w.,@?^=%&:\/~+#-*]*[\w@?^=%&\/~+#-*])"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would recognising file:// be desirable as well? Because major browsers support that as well

Copy link

@ToxicMushroom ToxicMushroom Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think compiling this regex with re.compile() once and storing it as a global might make it quicker, thought ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah good point

@TrigamDev
Copy link
Contributor Author

TrigamDev commented Jun 7, 2025

Disabled the Ruff "line too long" error (can't exactly add line breaks to the middle of a regex pattern) and added a trailing new line

@Computerdores
Copy link
Collaborator

I think technically you can (see re.VERBOSE) but its fine here imo

@TrigamDev
Copy link
Contributor Author

Turned out to be unnecessary regardless

@CyanVoxel
Copy link
Member

Are there any more changes desired for this implementation in the short term? Even if it doesn't cover 100% of use cases or could be done without using regex, it would probably be better to have at least some system providing clickable links than nothing at the moment

@CyanVoxel CyanVoxel moved this from 👀 In review to 🍃 Pending Merge in TagStudio Development Aug 4, 2025
@CyanVoxel CyanVoxel added the Status: Mergeable The code is ready to be merged label Aug 4, 2025
Copy link
Member

@CyanVoxel CyanVoxel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your work on this!

@CyanVoxel CyanVoxel merged commit c71032f into TagStudioDev:main Aug 7, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from 🍃 Pending Merge to ✅ Done in TagStudio Development Aug 7, 2025
@CyanVoxel CyanVoxel removed the Status: Mergeable The code is ready to be merged label Aug 7, 2025
@TrigamDev TrigamDev deleted the feat/clickable-links-in-text-fields branch August 8, 2025 03:05
@TrigamDev
Copy link
Contributor Author

Are there any more changes desired for this implementation in the short term? Even if it doesn't cover 100% of use cases or could be done without using regex, it would probably be better to have at least some system providing clickable links than nothing at the moment

Sorry, forgot to reply. Short-term, yes it's complete.

I think a good long-term solution for handling links could possibly be to include a simple UI-based markdown editor for long form text fields, such as descriptions. That way the user can specify exactly what is and isn't a link, among being able to use the rest of markdown's styling.
This is an entirely different feature of course, and I can open a separate issue for it.

(Discord's "Post Guidelines" input as an example for a simple UI for it)
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement New feature or request Type: QoL A quality of life (QoL) enhancement or suggestion Type: UI/UX User interface and/or user experience
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Add the ability to open the URL in the URL field
5 participants